I don't like this...

I think that in testing we could either do:

await Future.delayed(microseconds: 10);

Instead of waiting for deletionsDone.

Or we could change the tests to wait for deleteInstanceFn to have been called N times.

Or we could await deleteInstanceFn in here, with some short timeout like 30s and if it has happened then we log a warning. Nor sure this necessary.

Or we could make the agreement that deleteInstanceFn invocations will have been started before we return from scanAndDeleteInstances.

But we won't have awaited the futures from deleteInstanceFn invocations.

That way, if deleteInstanceFn does anything sync, the sync thing will have run.

Maybe, scanAndDeleteInstances could return ({DeleteInstancesState nextState, Future<void> done}).

I don't like having a type for this... I don't know why.

It feels like we're letting internals escape.

I think we should make the promise that deleteInstanceFn will not be invoked after the Future from scanAndDeleteInstances has been resolved.

We're not promising that Futures from deleteInstanceFn have been resolved, just that we won't make any more calls to deleteInstanceFn.

This requires us to not use scheduleMicrotask(..., but instead do:

Future? f;
try {
  // We've promised to never all deleteInstanceFn after the return
  // from this function! But have not promised to await Futures
  // from deleteInstanceFn in this function!
  f = deleteInstanceFn(...)
} catch (e, st) {
  _log.severe('Failed to delete $instance', e, st);
}
if (f != null) {
  scheduleMicroTask(() async {
    try {
      await f;
    } catch (e, st) {
      _log.severe('Failed to delete $instance', e, st);
    }
  });
}

or something like that.

We should declare the invariant in the documentation comment for scanAndDeleteInstances.